[Breaking Change] Update UDF validator to treat null/empty values as valid #80
Conversation
Update description for UDF validator once coldbox-modules/cbvalidation#80 goes live.
| if ( isNull( arguments.targetValue ) || isNullOrEmpty( arguments.targetValue ) ) { | ||
| return true; | ||
| } | ||
|
|
||
| // Validate via method |
There was a problem hiding this comment.
The only issue I see here, is that if arguments.targetValue is null, then line 46 will fail
There was a problem hiding this comment.
I'm not sure I follow. If arguments.targetValue is null, the method will return true, so it should never get to line 46.
| // Validate against the UDF/closure | ||
| var passed = arguments.validationData( | ||
| isNull( arguments.targetValue ) ? javacast( "null", "" ) : arguments.targetValue, | ||
| arguments.targetValue, |
There was a problem hiding this comment.
I am thinking that if the value is actually null, this will fail, did you try it?
There was a problem hiding this comment.
@lmajano I haven't tested it in production, but I updated the UDFValidatorTest.cfc lines 30-37 to account for passing a null value and the validator returns true. Do you want me to create a temporary repo to test the validator instead of relying on Testbox?
There was a problem hiding this comment.
Forget it, the way it was reading on my phone it was that lines 37-41 where removed.
|
Ok, can you make sure this works @homestar9 it's now in |
|
@lmajano tested |
Updated for consistency (see ortus-docs/cbvalidation-docs#13) and will treat null/empty values as valid. Developers who want to dynamically determine whether a field should be required will need to switch to using the
RequiredIfvalidator.Warning: This PR should be viewed as a breaking change and reserved for the next major version of CBValidation.